tensorrt_llm/executor/rpc/rpc_client.py (2)

254-271: Reader startup robustness: narrow exception and log stack.

Logic is good; avoid blind except Exception and log traceback for diagnosis.

-        except Exception as e:
-            logger_debug(f"Error starting response reader: {e}")
+        except (RuntimeError, asyncio.CancelledError) as e:
+            logger_debug(f"Error starting response reader: {e}")
+        except Exception:
+            logger.exception("Error starting response reader")
             # Don't raise here, let it retry on next call
             self._reader_task = None

Ruff BLE001.

---

359-359: Replace fixed sleep with readiness wait.

Poll is_running() with a short backoff and timeout instead of a magic 0.2s.

-            # Give the loop a moment to start
-            time.sleep(0.2)
+            # Wait for the loop to start running (max 1s)
+            waited = 0.0
+            while not self._loop.is_running() and waited < 1.0:
+                time.sleep(0.01)
+                waited += 0.01

tests/unittest/executor/test_rpc.py (4)

217-229: Test semantics: create pendings before shutdown.

This test now enqueues requests after triggering shutdown. To validate “pending requests are cancelled when server shuts down,” enqueue first, then shutdown.

-        client = RPCClient(addr)
-        try:
-            client.shutdown_server()
-            pending_futures = [client.task().remote_future() for _ in range(10)]
+        client = RPCClient(addr)
+        try:
+            pending_futures = [client.task().remote_future() for _ in range(10)]
+            client.shutdown_server()

---

284-297: Ensure deterministic server teardown.

After remote shutdown, explicitly call server.shutdown() locally to join the dispatcher instead of relying on sleep(1.0).

-    finally:
-        # Wait for the server dispatcher thread to quit
-        time.sleep(1.0)
+    finally:
+        # Join dispatcher deterministically
+        server.shutdown()

---

299-301: Track flaky test with an issue ID.

Add a reference (issue/URL) to the skip reason to avoid permanent skip.

-@pytest.mark.skip(reason="This test is flaky, need to fix it")
+@pytest.mark.skip(reason="Flaky; tracked in ISSUE-XXXX: fix perf timing nondeterminism")

---

375-393: Teardown order and sleeps.

Good: server first, then client. Prefer joining or polling over fixed sleep(1.0) to shorten tests and reduce flakiness.

-        # Wait longer to ensure all background threads exit completely
-        time.sleep(1.0)
+        # Optional: poll for thread shutdown instead of fixed sleep
+        # e.g., wait up to 1s in 20 * 50ms steps if available

tensorrt_llm/executor/rpc_proxy.py (4)

67-77: Avoid inner import; consider public starter.

• Remove inner import time; use the module-level import.
• Using a private _start_response_reader_lazily is brittle; consider exposing a public ensure_reader_started() on RPCClient later.

-        if hasattr(self.rpc_client, '_start_response_reader_lazily'):
-            self.rpc_client._start_response_reader_lazily()
-            # Give response reader time to start
-            import time
-            time.sleep(0.1)
+        if hasattr(self.rpc_client, '_start_response_reader_lazily'):
+            self.rpc_client._start_response_reader_lazily()
+            time.sleep(0.1)  # Give response reader time to start

---

298-320: Register result before submit; drop unused exception var and log.

• Great race fix moving registration before send.
• Remove unused e and log with traceback if needed.

-        except Exception as e:
+        except Exception:
             # Clean up on error
             self._results.pop(client_id, None)
-            raise
+            logger.exception("submit() failed")
+            raise

Ruff F841.

---

346-364: Shutdown: avoid blind except, prefer logger.exception.

Use specific exceptions where possible, or at least log stack traces.

-            except Exception as e:
-                logger.warning(f"Error during main loop shutdown: {e}")
+            except Exception:
+                logger.exception("Error during main loop shutdown")

-            except Exception as e:
-                logger.warning(f"Error during MPI session shutdown: {e}")
+            except Exception:
+                logger.exception("Error during MPI session shutdown")

-        except Exception as e:
-            logger.warning(f"Error during RPC client close: {e}")
+        except Exception:
+            logger.exception("Error during RPC client close")

Ruff BLE001.

---

369-373: Remove f-strings without placeholders.

Ruff F541: unnecessary f prefix.

-                logger_debug(f"Shutting down mpi session", color="yellow")
+                logger_debug("Shutting down mpi session", color="yellow")
-                logger_debug(f"Mpi session shutdown", color="yellow")
+                logger_debug("Mpi session shutdown", color="yellow")

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro